Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove ice roads from Finland routing #5355

Merged
merged 2 commits into from
Sep 14, 2023

Conversation

vesameskanen
Copy link
Contributor

Summary

A simple filtering of ice and winter roads using Finland tag mapping. Ice roads are in use only some months per year, so they should be exluded from the street graph.

At later stage, we might consider implementing a configurable time period for such roads, or perhaps start supporting some OSM tags which limit the time period when the roads are available.

@vesameskanen vesameskanen requested a review from a team as a code owner September 13, 2023 11:03
@vesameskanen vesameskanen added Skip Changelog HSL Helsinki Regional Transport Authority labels Sep 13, 2023
Copy link
Member

@leonardehrenfried leonardehrenfried left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is okay for me but I would encourage you to write a test with some examples.

The reason I'm saying this is that I did something similar in the HoustonMapper and I realised that other matches can "win" over yours.

For example if there is a matcher saying highway=residential;sidewalk=no it would win over your new ones because they describe a "better" match. I was very surprised by this.

This is not required for getting this merged.

@vesameskanen
Copy link
Contributor Author

Ok good point.

I was also wondering if this should be put in the default mapper. The problem is same everywhere. We do not have many users in places where ice is permanent. And those few probably won't use journey planners.

@t2gran t2gran added this to the 2.5 (next release) milestone Sep 13, 2023
@codecov
Copy link

codecov bot commented Sep 14, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.06% 🎉

Comparison is base (5c08a00) 66.38% compared to head (1a5a5ed) 66.44%.
Report is 21 commits behind head on dev-2.x.

Additional details and impacted files
@@              Coverage Diff              @@
##             dev-2.x    #5355      +/-   ##
=============================================
+ Coverage      66.38%   66.44%   +0.06%     
- Complexity     15196    15211      +15     
=============================================
  Files           1786     1786              
  Lines          69232    69239       +7     
  Branches        7319     7311       -8     
=============================================
+ Hits           45957    46005      +48     
+ Misses         20799    20757      -42     
- Partials        2476     2477       +1     
Files Changed Coverage Δ
...lanner/openstreetmap/tagmapping/FinlandMapper.java 91.39% <100.00%> (+1.39%) ⬆️

... and 11 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@vesameskanen vesameskanen merged commit d6fb013 into opentripplanner:dev-2.x Sep 14, 2023
@vesameskanen vesameskanen deleted the no-ice-roads branch September 14, 2023 08:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
HSL Helsinki Regional Transport Authority Skip Changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants